-
Notifications
You must be signed in to change notification settings - Fork 900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Collect allocated storage for container projects #15973
Conversation
79a89a1
to
fc357cd
Compare
fc357cd
to
e215b22
Compare
@@ -0,0 +1,17 @@ | |||
module Metric::ContainerStorage | |||
def self.fill_allocated_container_storage(obj) | |||
volumes_or_claims = if obj.kind_of? ContainerGroup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC @nimrodshn added pod => pvc relation too, so it might be simpler to use it here.
ce28029
to
0060000
Compare
return {} if !obj.kind_of?(ContainerProject) | ||
|
||
sum_storage = obj.persistent_volume_claims.inject(0) do |sum, volume| | ||
sum + volume.capacity[:storage] if volume.capacity.present? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zakiva should this also check if the pvc is bounded or not?
I think so since only then it would mean that the project was allocated a pv(else its just a request?).
https://kubernetes.io/docs/concepts/storage/persistent-volumes/#lifecycle-of-a-volume-and-claim
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is no need to check because capacity represents the actual resources of the bounded volume. In case the claim is not bounded, capacity will be an empty hash.
@miq-bot remove_label wip |
0060000
to
be3dc46
Compare
Checked commit zeari@be3dc46 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
||
it "calculates container storage when having zero persistent volume claims" do | ||
derived_columns = described_class.fill_allocated_container_storage(project3) | ||
expect(derived_columns[:derived_vm_allocated_disk_storage]).to eq(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: projects with no pvc's or unbounded pvcs will have zero instead of nil
.
@simon3z Ping |
@zeari please update patch to use recently added collected PVC requests. |
No and no, since we dont collect metrics for project directly. AFAIK We have only metric rollups for |
Closing until relevant again. |
Fill in
derived_vm_allocated_disk_storage
in hourlyMetricRollups
for container groups and container projects.@cben @moolitayer @zakiva please review.
cc @simon3z
@miq-bot add_label providers/containers, wip, enhancement, providers/metrics
Edit: Storage for pods was removed from this PR since in the case of many pods using the same pvc, no one specific pod was allocated the entire storage capacity.